The implementation uses truthiness checks (!oldText, !newText) rather than explicit null/undefined checks. This causes empty strings to be treated the same as null/undefined, which is semantically incorrect:

• Empty file ("") vs null (non-existent file) have different meanings
• computeLineDiff("", "hello") incorrectly treats this as creating a new file, when it should show adding "hello" to an existing empty file

Change the checks to be explicit:

if (oldText === null || oldText === undefined) {
  // New file case
}
if (newText === null || newText === undefined) {
  // Deleted file case
}

This also aligns with the type definition that allows string | null | undefined and the ACP protocol specification mentioned in issue #43 which uses null for new/deleted files.

When renderDiff() output is assigned to tool.output (line 1004-1008), it will later be processed by getToolsHtml() which wraps all output in <pre class="tool-output"> tags (around line 360). Since renderDiff() already returns a complete HTML structure with <div class="diff-container">, this creates nested containers with semantic and styling issues.

The fix for the HTML escaping issue (comment #1) should also address this wrapping problem, likely by detecting when tool.output contains pre-rendered HTML and handling it differently in getToolsHtml().

The HTML output from renderDiff() will be double-escaped when processed by getToolsHtml(). In getToolsHtml() at lines 354-357, if the output doesn't contain ANSI codes, it calls escapeHtml() on the output. Since diff HTML doesn't contain ANSI codes, it will be escaped, causing users to see raw HTML tags instead of the rendered diff.

To fix this, the code needs a way to indicate that tool.output contains pre-rendered HTML. Options include:

1. Add a flag to the Tool interface (e.g., outputIsHtml: boolean)
2. Check for specific markers in the output string (e.g., check if output starts with <div class="diff-container">)
3. Store the content type alongside the output and check it in getToolsHtml()

The same pattern is used for ANSI terminal output (ansiToHtml returns HTML), but that works because hasAnsiCodes() detects the ANSI escape sequences in the original output before it's converted.

The current diff algorithm marks ALL old lines as removed and ALL new lines as added, which doesn't provide a useful diff visualization. For example, if only one line changes in a 100-line file, this will show 200 lines (100 deletions + 100 additions) instead of showing the single changed line with context.

While the comment at line 258 acknowledges this is a "simple algorithm" with "Future optimization" planned, this makes the diff feature almost unusable for real files. Even a basic longest common subsequence (LCS) algorithm would provide significantly better results by identifying unchanged lines to show as context.

Consider implementing at least a basic diff algorithm that identifies common lines, or document this limitation prominently in the PR description and user-facing documentation.

The diff rendering has its own truncation at 500 lines (line 280), but getToolsHtml() also truncates tool.output at 500 characters (line 351-352). This creates inconsistent behavior:

1. For a diff with 600 lines, renderDiff() produces HTML for 500 lines + truncation message
2. Then getToolsHtml() truncates this HTML string to 500 characters and appends "..."
3. The result is broken HTML showing only a fragment of the diff container

This dual truncation needs to be reconciled. Either remove the character-based truncation for HTML output, or ensure the line-based truncation in renderDiff() produces output that stays well under the character limit.

The DiffLine interface includes a "context" type in the union (line 227), but this type is never actually used in computeLineDiff(). The function only ever creates diff lines with type "add" or "remove".

Either remove "context" from the type definition, or implement context line detection in the diff algorithm. The presence of unused type options suggests incomplete implementation and could confuse future maintainers.

The test "returns empty array for empty inputs" passes empty strings to computeLineDiff, but the actual implementation treats empty strings as falsy and returns an empty array (line 240). However, empty strings ("") and null/undefined have different semantics - an empty string could represent an empty file, while null typically represents a non-existent file.

The current implementation conflates these cases: computeLineDiff("", "content") returns the same result as computeLineDiff(null, "content"), even though the first should show adding content to an existing empty file, while the second represents creating a new file.

Consider changing the logic to only check for null/undefined, not truthiness, to properly handle empty files.

The test suite is missing integration tests that verify diff HTML output works correctly when passed through getToolsHtml(). The current tests only verify:

1. renderDiff() produces HTML with the correct classes (tests at lines 1299-1340)
2. getToolsHtml() handles ANSI terminal output (tests at lines 1103-1161)

But there's no test that verifies:

const tools = {
  "tool-1": {
    name: "edit_file",
    output: renderDiff("/path/file.ts", "old", "new"),
    ...
  }
};
const html = getToolsHtml(tools);
// Verify the diff HTML is rendered correctly, not escaped

This missing test would have caught the critical HTML escaping bug where diff output gets double-escaped.